Skip to content

Fix completions of exports elsewhere in same file#43755

Merged
andrewbranch merged 5 commits intomicrosoft:masterfrom
andrewbranch:bug/43289
Apr 26, 2021
Merged

Fix completions of exports elsewhere in same file#43755
andrewbranch merged 5 commits intomicrosoft:masterfrom
andrewbranch:bug/43289

Conversation

@andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Apr 20, 2021

Fixes #43289, along with another bug I found on the way:

module.exports = { a/**/ } // has a completion for itself

and

const c = 0;
export { c as something };
export * as somethingElse from "./other";

s/**/ // has completions for 'something' and 'somethingElse'

@andrewbranch andrewbranch requested a review from sandersn April 21, 2021 00:08
sandersn
sandersn requested changes Apr 22, 2021
View reviewed changes
@andrewbranch
Copy link
Member Author

andrewbranch commented Apr 23, 2021 •
edited
Loading

@sandersn I realized I couldn’t really fix the primary bug (#43289) without addressing the contextual typing thing head-on, which I think I’ve done now. getContextualTypeForThisPropertyAssignment is now only used for this (and aliased-this) property assignments. We now group AssignmentDeclarationKind.ExportsProperty, .Prototype, .PrototypeProperty, and .ModuleExports together as being the simpler cases where they can have a contextual type only via a JSDoc type annotation, whereas .Property and .ThisProperty might be new declarations, might be immediately annotated with JSDoc, or might have a contextual type from the original declaration of the object/class. Some of the simpler cases were incorrectly being fed through that more complicated logic.

@andrewbranch andrewbranch requested a review from sandersn April 23, 2021 20:28
@andrewbranch
Copy link
Member Author

andrewbranch commented Apr 23, 2021

@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 23, 2021 •
edited
Loading

Heya @andrewbranch, I've started to run the parallelized community code test suite on this PR at 61521db. You can monitor the build here.

@typescript-bot typescript-bot mentioned this pull request Apr 23, 2021
Open
@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 23, 2021

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

sandersn
sandersn approved these changes Apr 24, 2021
View reviewed changes
src/compiler/checker.ts
case AssignmentDeclarationKind.ModuleExports:
case AssignmentDeclarationKind.ThisProperty:
return getContextualTypeForThisPropertyAssignment(binaryExpression, kind);
valueDeclaration ||= binaryExpression.symbol?.valueDeclaration;
Copy link
Member

@sandersn sandersn Apr 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch scope rules are boooooooooo

@andrewbranch andrewbranch merged commit b9c1e98 into microsoft:master Apr 26, 2021
@andrewbranch andrewbranch deleted the bug/43289 branch April 26, 2021 16:13
@jakebailey jakebailey mentioned this pull request Aug 4, 2022
Merged
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn sandersn sandersn approved these changes

@RyanCavanaugh RyanCavanaugh RyanCavanaugh approved these changes

Assignees

@andrewbranch andrewbranch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Completions broken in module.exports object literal

4 participants

@andrewbranch @typescript-bot @sandersn @RyanCavanaugh